-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[translation] adds docstrings and code snippets #17642
Conversation
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_client.py
Outdated
Show resolved
Hide resolved
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_client.py
Outdated
Show resolved
Hide resolved
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_client.py
Outdated
Show resolved
Hide resolved
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_client.py
Outdated
Show resolved
Hide resolved
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_client.py
Show resolved
Hide resolved
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_models.py
Outdated
Show resolved
Hide resolved
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_models.py
Show resolved
Hide resolved
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_models.py
Outdated
Show resolved
Hide resolved
sdk/documenttranslation/azure-ai-documenttranslation/azure/ai/documenttranslation/_models.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really like your docstrings
:rtype: ~azure.core.polling.ItemPaged[JobStatusResult] | ||
"""List all the submitted translation jobs under the Document Translation resource. | ||
|
||
:keyword int top: Use top to indicate the total number of results you want |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my understanding was you guys weren't going to expose top
and skip
for this release, I could be wrong though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct, will remove in future PR
this is sorted by descending start time. | ||
:keyword int results_per_page: Use results_per_page to indicate the maximum number | ||
of results returned in a page. | ||
:return: ItemPaged[:class:`~azure.ai.documenttranslation.JobStatusResult`] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is ItemPaged
rendering correctly in sphinx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed. I think I had it like that to appease Apiview
use the default_version for the file format returned from the | ||
:func:`~DocumentTranslationClient.get_glossary_formats()` client method. | ||
:keyword str storage_source: Storage Source. Default value: "AzureBlob". | ||
Currently only "AzureBlob" is supported. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you need to add ivars
here? Idk if users would actually access the properties, but you can
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree that it's unlikely a user would access them, but will add :ivars: to be more complete
inner error with more descriptive details. | ||
:ivar str status: Status for a job. | ||
|
||
* `NotStarted` - the job has not begun yet. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A little out of this scope, but have you guys thought about making this an enum?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only returned, never used as an input. If we made it an enum it would be purely for documentation purposes... last we had this conversation we decided that it should just be string.
Resolves #17046 #17047